-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keep existing url config in utoipa-swagger-ui #274
Conversation
It has been designed so that for So why just not like this? SwaggerUi::new("/v1/swagger/*tail")
.url("/v1/openapi.json", openapi); Also in a same way the you could have only SwaggerUi::new("/swagger/*tail")
.url("/v1/openapi.json", openapi); It originally was not designed so that someone would define the paths separately with additional config thus it just overrides the config urls with the |
I think this issue is caused by axum's My application is like below. let swagger = SwaggerUi::new("/swagger/*tail")
.url("/openapi", openapi);
let services = Router::new()
.route("/", get(root))
.route("/users", get(list_users))
.merge(swagger);
let app = Router::new()
.nest("/v1", services) // This makes all endpoints start from "/v1" prefix
.layer(Extension(db_connection_pool));
let addr = SocketAddr::from(([0, 0, 0, 0], 3000));
axum::Server::bind(&addr)
.serve(app.into_make_service())
.await
.unwrap(); Because of If you set Like this, I feel it's better to split Of course, I can achieve this design by merging swagger route with avoiding Router::new()
.nest("/v1", services)
.merge(swagger) Maybe this issue can be resolved by describing this specification in docs without changing |
Oh yeah, true it does not work since the
This looks to me quite elegant solution, and if it works it could be mentioned in docs. There also could be an example in examples folder as well regarding this. I'm not opposing the change, it's just that I feel like there is some room for an improvement on how users define the url and openapi mapping. If there are multiple definitions of single url it's hard to tell which one should be the authoritative one. |
@komi1230 I was thinking about this change and it is reasonable to make. So after next release you also split the configuration as you originally intented. 🙂
I agree, I continue on this branch to make this change for other frameworks as well. |
Hello,
TL; DR
I found a bug in axum extension about
url
config.So far if you set url config, the config could be overridden.
This PR resolves this by checking
url
andurls
field beforehand.Background
In my application, API is versioned and each endpoint has a prefix of
/v1
.Axum has
nest
method to roll up endpoints, and I can achieve this API versioning.About Swagger UI, swagger's endpoint also has to starts from
/v1
and this is easily achieved by belowBut swagger will read
/openapi.json
so I have to change this to/v1/openapi.json
.I have to set
url
config to make Swagger UI accessible in endpoint/v1/swagger
and I wrote below code.But this has been overridden.
Caveats
I wrote in above, axum has
nest
method and can roll up routes.To resolve my issue, I decided to attach
url
config toSwaggerUI
but I feel it's also OK to addbase_path
field toConfig
and automatically apply base path toSwaggerUIBundle
.If you like the latter approach, I will resubmit PR.
Thank you for checking.